RFC: Add Emoji support to denom #6744
Conversation
okwme
requested review from
alessio,
alexanderbez,
fedekunze and
jackzampolin
as code owners
Codecov Report
@@ Coverage Diff @@
## release/v0.38.4 #6744 +/- ##
==================================================
Coverage ? 51.50%
==================================================
Files ? 337
Lines ? 20617
Branches ? 0
==================================================
Hits ? 10619
Misses ? 9195
Partials ? 803 |
|
@okwme this needs to be rebased to master, and once merged, cherry-picked to the release branch |
|
@fedekunze dixit:
True that. Plus, I'd like to see some test cases. It's a fun idea, but perhaps instead of amending the regular expression we should finally find a way to allow client apps to implement their own denom validation functions. |
ooh i like that idea @alessio. What do you think that would ideally look like? |
diff --git a/types/coin.go b/types/coin.go
index 96b9c71b2..9c7ec9cdb 100644
--- a/types/coin.go
+++ b/types/coin.go
@@ -589,15 +589,18 @@ var (
reDecCoin = regexp.MustCompile(fmt.Sprintf(`^(%s)%s(%s)$`, reDecAmt, reSpc, reDnmString))
)
-// ValidateDenom validates a denomination string returning an error if it is
-// invalid.
-func ValidateDenom(denom string) error {
+// DefaultValidateDenom is the default validation function for Coin.Denom.
+func DefaultValidateDenom(denom string) error {
if !reDnm.MatchString(denom) {
return fmt.Errorf("invalid denom: %s", denom)
}
return nil
}
+// ValidateDenom validates a denomination string returning an error if it is
+// invalid.
+var ValidateDenom = DefaultValidateDenom
+
func mustValidateDenom(denom string) {
if err := ValidateDenom(denom); err != nil {
panic(err)Something like that would allow the client app to customise the |
|
Closing in favor of a PR for custom validation against |
|
My fault, this is a PR - not an issue. Hence closing again. |
Allow ValidateDenom to be customized per application. closes: #6744
Description
This PR adds support for emojis to the coin type's denom. It adds a whitelist of UTF8 character ranges that encompass the basic unicode 3.0 emoji list (http://unicode.org/Public/emoji/3.0/emoji-data.txt).
I'm not sure what the original reasoning was for limiting denoms to lowercase alphanumeric characters between 3 and 16 in length. This maintains that limitation while adding exceptions for emojis. I know this is a silly PR but would be curious to hear exactly why this might not be a bad idea before closing it. Otherwise if there are no strong reasons it could benefit the SDK to be emoji compatible as a PR stunt or to encourage more playful tutorials (currently working on a burner chain that uses emojis as a currency).
closes: #XXXX
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes